Social previews: Show skeleton while rendering messages#48576
Social previews: Show skeleton while rendering messages#48576
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryCoverage changed in 9 files. Only the first 5 are listed here.
Full summary · PHP report · JS report Coverage check overridden by
Coverage tests to be added later
|
manzoorwanijk
left a comment
There was a problem hiding this comment.
This works well, but we can improve the code a bit more.
| void registry | ||
| .resolveSelect( socialStore ) | ||
| .getRenderedMessages( postId, items ) | ||
| // Errors are intentionally swallowed to preserve existing UI behavior. | ||
| .catch( () => {} ); |
There was a problem hiding this comment.
Is this to fix any issue we faced while testing?
…ew-skeleton # Conflicts: # projects/packages/publicize/_inc/hooks/use-connection-preview-data/index.ts # projects/packages/publicize/_inc/hooks/use-render-message-items/index.ts
The consumer was stitching loading from three signals — `isResolving`,
`hasFinishedResolution`, and a derived `isWaitingForRenderedMessage` —
to fill the gap between an items-array commit and the resolver actually
finishing. Move that bookkeeping into the slice where it belongs.
Each cache entry is now `{ isLoading: boolean; items?: RenderedMessageBatch }`.
The resolver dispatches `startRenderingMessages` before the apiFetch,
`receiveRenderedMessages` on success, and `finishRenderingMessages` in
the error path (clears `isLoading` without overwriting any prior items,
preserving the "no flash on failure" semantics).
The selector `isLoadingRenderedMessages` returns true when either the
resolver has explicitly marked the slot loading or the entry doesn't
exist yet — the "no entry yet" window covers the gap between debounce
flush and resolver `start`, closing the brief flash where the preview
showed the raw baseMessage.
Consumer collapses to:
isLoading = templatesEnabled && (isDebouncing || isLoadingRendered)
The debounce check stays in the consumer because the store doesn't see
edits until items are committed — pushing it into the slice would mean
tracking a "latest pending hash" alongside the cache, which is a bigger
change for a small win.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…in items useRenderMessageItems was checking `hasConnectionMessage` before mode, so items[i].message kept the old per-network customization even after the user switched back to global. baseMessage in useConnectionPreviewData is already mode-gated, so the two sides diverged — `currentRenderItem.message` never matched `baseMessage` and `isDebouncingRenderedMessage` stayed stuck true, freezing the preview on the skeleton. Mirror the consumer's rule exactly: per-connection message + template only contribute in per-network mode; otherwise globalMessage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixes SOCIAL-475
Proposed changes
Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
{excerpt}into the global or per-connection message template./wpcom/v2/publicize/render-messagesrequest fires after typing pauses.pnpm --filter @automattic/social-previews test.pnpm --filter @automattic/social-previews typecheck.pnpm --filter @automattic/jetpack-publicize test -- use-connection-preview-data use-render-message-items.pnpm --filter @automattic/jetpack-publicize typecheck.CleanShot.2026-05-07.at.11.51.16.mp4